Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ensure that the built PdfJsDefaultPreferences.jsm file won't be affected/touched during tree-wide ESLint rule changes in mozilla-central (PR 9571 follow-up) #10020

Merged
merged 2 commits into from
Aug 29, 2018

Conversation

Snuffleupagus
Copy link
Collaborator

Also updates the edit warning, such that the wording is more consistent.

@@ -751,15 +751,17 @@ function preprocessDefaultPreferences(content) {
var preprocessor2 = require('./external/builder/preprocessor2.js');
var licenseHeader = fs.readFileSync('./src/license_header.js').toString();

var GLOBALS = '/* eslint-disable */\n';
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't \n be in front of the string here, because otherwise there is no newline between the licenseHeader and GLOBALS below? Alternatively, we can remove \n from this string altogether and just join the parts below in the return with \n.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't \n be in front of the string here, because otherwise there is no newline between the licenseHeader and GLOBALS below?

There usually isn't a newline between license headers and "globals" in PDF.js code, see e.g.

pdf.js/web/app.js

Lines 12 to 15 in 436d2ef

* See the License for the specific language governing permissions and
* limitations under the License.
*/
/* globals PDFBug, Stats */

which is why I just didn't bother inserting one here either. However, I'm changing this as suggested, since it cannot hurt to be consistent below when the strings are joined :-)

Alternatively, we can remove \n from this string altogether and just join the parts below in the return with \n.

That would require adding two back-to-back newlines when joining the strings below, which would look slightly out of place in my opinion. (The trailing \n chars just moves to the next line, the \n when producing the final string is what gives the blank lines.)


Anyway, with the slightly tweaked patch, this is how the output looks now:

/* Copyright 2018 Mozilla Foundation
 *
 * Licensed under the Apache License, Version 2.0 (the "License");
 * you may not use this file except in compliance with the License.
 * You may obtain a copy of the License at
 *
 *     http://www.apache.org/licenses/LICENSE-2.0
 *
 * Unless required by applicable law or agreed to in writing, software
 * distributed under the License is distributed on an "AS IS" BASIS,
 * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
 * See the License for the specific language governing permissions and
 * limitations under the License.
 */

/* eslint-disable */

//
// THIS FILE IS GENERATED AUTOMATICALLY, DO NOT EDIT MANUALLY!
//

"use strict";
var EXPORTED_SYMBOLS = ["PdfJsDefaultPreferences"];
var PdfJsDefaultPreferences = Object.freeze({
 "showPreviousViewOnLoad": true,
 "defaultZoomValue": "",
 "sidebarViewOnLoad": 0,
 "cursorToolOnLoad": 0,
 "enableWebGL": false,
 "pdfBugEnabled": false,
 "disableRange": false,
 "disableStream": false,
 "disableAutoFetch": false,
 "disableFontFace": false,
 "textLayerMode": 1,
 "useOnlyCssZoom": false,
 "externalLinkTarget": 0,
 "renderer": "canvas",
 "renderInteractiveForms": false,
 "enablePrintAutoRotate": false,
 "disablePageMode": false,
 "disablePageLabels": false,
 "scrollModeOnLoad": 0,
 "spreadModeOnLoad": 0
});

…ected/touched during tree-wide ESLint rule changes in `mozilla-central` (PR 9571 follow-up)

Also updates the edit warning, such that the wording is more consistent.
@Snuffleupagus Snuffleupagus force-pushed the addon-prefs-no-eslint branch from 7b160aa to d8aaa2f Compare August 28, 2018 21:52
@timvandermeij
Copy link
Contributor

/botio-windows preview

@pdfjsbot
Copy link

From: Bot.io (Windows)


Received

Command cmd_preview from @timvandermeij received. Current queue size: 0

Live output at: http://54.215.176.217:8877/3815112987c0c0a/output.txt

@pdfjsbot
Copy link

From: Bot.io (Windows)


Success

Full output at http://54.215.176.217:8877/3815112987c0c0a/output.txt

Total script time: 4.28 mins

Published

@timvandermeij timvandermeij merged commit 27ebb41 into mozilla:master Aug 29, 2018
@timvandermeij
Copy link
Contributor

Looks good. Thank you for the follow-up!

@Snuffleupagus Snuffleupagus deleted the addon-prefs-no-eslint branch August 29, 2018 20:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants